Skip to content

fix: normalize peer address before duplicate-identity comparison - #48

Merged
torlando-tech merged 5 commits into
mainfrom
fix/ble-dup-identity-mac-normalize
Oct 1, 2026
Merged

torlando-tech merged 5 commits into
mainfrom
fix/ble-dup-identity-mac-normalize

Conversation

@torlando-tech

Copy link
Copy Markdown
Owner

Summary

On two fixed-MAC peers both running central + peripheral mode, the same physical peer is represented by two address strings: the peripheral (GATT) path stores dev:AA:BB:.. (BlueZ D-Bus device-path form) while the central (scan) path carries the bare AA:BB:... The _check_duplicate_identity comparison saw dev:AA:BB != AA:BB, concluded a false Android MAC rotation, rejected the connection, and detached the peer interface after the grace period.

The data path then dropped: discovery announces routed to 0 peers and the peer never appeared in the destination table.

Fix

Add _normalize_address() that strips the dev: prefix and case-folds. Apply it to both comparison sites in _check_duplicate_identity and the v2.2 scan loop. Genuinely different MACs (true Android MAC rotation) still differ and are still handled.

TDD

test_ble_dup_identity_mac_normalize.py reproduces the false positive (RED before, GREEN after) and guards that true MAC rotation is still rejected. Bind the new _normalize_address helper in the zombie/blacklist test harnesses that call the real method.

Test results

330 passed, 6 failed (all pre-existing: No module named 'dbus', fail identically on clean main).

On two fixed-MAC peers both running central + peripheral, the same
physical peer is represented by two address strings: the peripheral
(GATT) path stores "dev:AA:BB:.." (BlueZ D-Bus device-path form) while
the central (scan) path carries the bare "AA:BB:..". The
_check_duplicate_identity comparison (and the v2.2 scan-loop comparison)
saw "dev:AA:BB" != "AA:BB", concluded a false Android MAC rotation,
rejected the connection, and detached the peer interface after the grace
period. The data path then dropped: discovery announces routed to 0
peers and the peer never appeared in the destination table.

Strip the "dev:" prefix and case-fold before comparing, so the same
physical MAC compares equal regardless of which form it arrived in,
while genuinely different MACs (true MAC rotation) still differ.

TDD: test_ble_dup_identity_mac_normalize.py reproduces the false
positive (RED before, GREEN after) and guards that true MAC rotation is
still rejected. Bind the new _normalize_address helper in the
zombie/blacklist test harnesses that call the real method.
@codecov

codecov Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@greptile-apps

greptile-apps Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Medium risk] Fixes MAC address comparison in peer duplicate detection.

The PR appears safe to merge; no new blocking issue was found.

What we checked:

  • Scan tests miss the branch: The tests give the scanned peer a known identity and an existing interface, then check whether the peer is selected.
  • New tests miss coverage: The workflow runs the tests/ directory and does not ignore the new file. It ignores test_v2_2_mac_sorting.py instead.

Summary

BLE identity checks now normalize addresses before comparing them, so dev: and bare forms of one MAC match across central and peripheral paths. Genuinely different MACs still follow the existing rotation rules.

  • Removes the dev: prefix, trims spaces, and folds letter case.
  • Adds coverage for matching address forms and genuine MAC changes.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Scan finds a known identity] --> B{Normalized addresses match?}
  B -->|Yes| C[Skip existing interface]
  B -->|No| D{Old connection alive?}
  D -->|Yes| E[Skip new address]
  D -->|No| F[Clean up old address and select new peer]
Loading

Reviews (4) · Last reviewed commit: "test: cover scan-loop address-normalizat..."

Comment thread src/ble_reticulum/BLEInterface.py
The normalized same-address branch in _select_peers_to_connect (dev:-prefix
peripheral form vs bare central form for the same fixed-MAC peer) is now
pinned by TestScanLoopSameAddressRegression: the skip decision (interface
already exists, so the peer is not re-added as a 'MAC rotation') is asserted,
with a stale-interface (not-in-self.peers) state, and a contrast test proving
that the skip is caused by the branch rather than another gate.

Also documents that the same-MAC reconnect path is the standard top-level gate
(address in self.peers), not this branch.
@torlando-tech

Copy link
Copy Markdown
Owner Author

@greptile review

Android (AndroidBLEInterface) and iOS subclass the shared BLEInterface and
wire on_duplicate_identity_detected -> _check_duplicate_identity with no
override. Their drivers produce bare, same-case MACs (never the BlueZ dev:
prefix that is the headline Linux fix), so the normalizer must be a no-op for
the common mobile case and must not disable true MAC-rotation rejection.

Add TestBareMacMobileSafety to pin that contract on the real method:
  * bare same-case MAC, alive  -> not a duplicate (unchanged)
  * bare different MAC, alive  -> still rejected (guard holds)
so a future edit cannot silently alter Android/iOS behavior.
@torlando-tech

Copy link
Copy Markdown
Owner Author

@greptile review

Comment thread src/ble_reticulum/BLEInterface.py Outdated
…nitpick)

The TestScanLoopSameAddressRegression class lives in tests/test_v2_2_mac_sorting.py
(not test_ble_dup_identity_mac_normalize.py); fix the pointer in the code comment.
@torlando-tech

Copy link
Copy Markdown
Owner Author

@greptile review

The v2.2 same-identity normalization in _select_peers_to_connect is pinned
by tests/test_v2_2_mac_sorting.py, but that file is excluded from the
integration coverage run (--ignore=...), so codecov/patch sees the changed
scan-loop lines as uncovered and fails.

Add tests/test_scan_loop_normalize_coverage.py that drives the REAL
_select_peers_to_connect from an included module, pinning each branch
(rotation-alive skip, rotation-dead reselect + cleanup, same-normalized
address skip) plus the _normalize_address falsy-input branch. Verified:
the three changed BLEInterface regions (check_duplicate_identity,
scan-loop norm, _normalize_address) now show 0 uncovered lines under the
CI-equivalent coverage run.
@torlando-tech
torlando-tech merged commit 7dc0edf into main Oct 1, 2026
14 checks passed
@torlando-tech
torlando-tech deleted the fix/ble-dup-identity-mac-normalize branch October 1, 2026 02:06
torlando-tech added a commit that referenced this pull request Oct 3, 2026
Adds the Keep-a-Changelog entry for the v0.2.3 release covering the
scanner wedge health-check, the ifac_size inheritance fix that was
dropping all inbound BLE packets, announce-rate stats, duplicate-
identity normalization, the package rename, and related fixes (#45,
#47, #48, #49 plus the #29-#44 window).

This was the missing precondition blocking the v0.2.3 release workflow:
the tag points at a commit where pyproject.toml still reads 0.2.2 and
no CHANGELOG.md entry exists.

Co-authored-by: torlando-tech <torlando-tech@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant